Cache the apt archives, and verify the cache is used (#386) - #410
Conversation
…oads anyway The measurement on commandprompt#386: across the last 80 runs, 352 successes of this step at a median of 25s, and six failures all pinned at 606 to 608s. Nothing in between. The step either finishes in half a minute or hits the 600s bound. The stall is azure.archive.ubuntu.com serving the LLVM toolchain at around 30 kB/s. That toolchain is 91 MB of the 102 MB and is a hard dependency: $ apt-cache depends postgresql-server-dev-18 Depends: clang-21 Depends: llvm-21-dev so --no-install-recommends cannot drop it, and there is no reason to want to. It is a legitimate dependency. The problem is downloading it repeatedly across a mirror that intermittently will not serve it. apt now writes its archives to a cached directory, keyed on the PG major, the runner OS and arch, and ImageOS. ImageOS is in the key because a new runner image can bring a different LLVM major, and a key that ignored it would serve debs for the wrong one. The second half matters more than the first. A cache that populates and never hits looks identical in the log to one that works: the job is simply still slow, and nothing reports it. So on a reported hit, the step now fails if apt fetched anything over the network. That is the same defect shape as commandprompt#396, where a suite was exempt from the registration check and never invoked. Two things deliberately not done, both from the issue: Raising the bound. At 30 kB/s the fetch needs roughly 50 minutes, which trades a red check for a runner held most of an hour. Pinning a different mirror. The runner chooses from its own mirrorlist, and betting on a different one is the same bet we are losing now. A cache miss degrades to exactly today's behaviour, and a stale entry is re-fetched by apt rather than used, so the failure mode is a slow job and not a wrong one.
jdatcmd
left a comment
There was a problem hiding this comment.
The diagnosis is right and the assertion instinct is right. Two problems in the
mechanism, and the greens do not yet mean what they look like.
"A cache that populates but never hits looks identical in the log to one that works: the
job is simply still slow. So assert it." That is exactly the right reflex, and it is the
reason I read the rest closely.
1. Every green on this PR is from the MISS path
$ gh run list --branch fix/386-apt-cache
30966218448 build and test success 2026-08-05T01:23:02Z
One run. A PR's first run is a cache miss by definition, so it populated the cache and
took the install path unchanged. The hit path has never executed, which means the
restore, the chown, and the assertion itself are all untested. Eleven green checks
here are evidence that a miss still works, which was never in doubt.
Re-run that workflow. The second run restores what the first saved, and then the greens
cover the path the PR is about. I do not think that is optional for a change whose whole
subject is that a broken cache is invisible.
2. The assertion turns the next PostgreSQL minor release into a red CI
The key is fixed:
key: apt-pgdev-codecs-pg${{ matrix.pg }}-...-v1
Nothing in it tracks the package versions. actions/cache skips the save step on an
exact-key hit, so once a key is populated its contents are frozen until a human edits
-v1.
Now PGDG publishes 17.6. apt-get update fetches the new lists, the cache hits, the
archives hold 17.5, apt resolves to 17.6, does not find it locally, and downloads it:
Get:1 https://apt.postgresql.org/... postgresql-17 17.6-1.pgdg22.04+1
cache-hit == true and a Get: line, so the assertion fires and exits 1. Every job
in the matrix goes red on the day of a routine upstream minor, and stays red until
someone bumps the key. That is a false red of exactly the kind we keep getting bitten by,
and this one is on a monthly-ish timer.
The fix is to put the resolved versions in the key rather than to weaken the assertion.
After apt-get update, in its own step:
- id: aptkey
run: echo "k=$(apt-cache policy postgresql-${{ matrix.pg }} postgresql-server-dev-${{ matrix.pg }} \
liblz4-dev libzstd-dev zlib1g-dev | md5sum | cut -c1-12)" >> "$GITHUB_OUTPUT"with key: ...-${{ steps.aptkey.outputs.k }} and a restore-keys: prefix. A version bump
becomes a new key, so it misses, downloads, and saves, and an exact hit then genuinely
must not download. The assertion keeps its teeth and stops being a tripwire.
3. || true on the chown recreates the failure class you are guarding against
sudo chown -R "$USER" "$HOME/apt-archives" || trueapt downloads as _apt, so partial/ and the debs are not owned by the runner user, and
without this the post-job save cannot read them. If the chown fails, the save fails, the
cache is never populated, there is never a hit, and the assertion never fires because
it is gated on cache-hit == true. Silent permanent misses: the job is simply still
slow, which is the sentence in your own comment.
Drop the || true, or assert the directory is readable afterwards. If there is a reason
it can fail benignly, that reason belongs in the comment.
Smaller, not blocking
- Four cache entries across the matrix at roughly 102 MB each, times five majors, is on
the order of 1.5 GB against the repo's 10 GB budget. Fine, but least-recently-used
eviction means a quiet major can miss anyway. Not a correctness issue, worth knowing
before someone reads a miss as a bug. set -euo pipefailis already set, soapt-get ... | teekeeps apt's exit status
throughpipefail. Good, and it is the kind of thing worth a word in the comment
since theteeis new.- Putting
ImageOSin the key for the LLVM major is a nice catch, and the comment
explaining why is the reason I did not have to work it out.
Points 1 and 2 are what I would want fixed before merge. Point 3 is a one-word change.
…mmandprompt#386) Three problems, all of which turn this cache into the thing it is meant to prevent: slow and green. The key tracked package names and not versions. actions/cache skips its save on an exact-key hit, so a fixed key freezes the archives until a human edits it. The day PGDG publishes a new minor, the cache hits with the old debs, apt resolves to the new ones and downloads them, and the download assertion sees a hit together with a Get: line and fails. Every matrix entry goes red on a routine upstream release and stays red until someone bumps the key by hand. That is a false red on a monthly timer, and weakening the assertion would be the wrong fix. The key now hashes the candidate versions apt resolves, so an upstream bump becomes a new key: it misses, downloads, and saves. A restore-keys prefix seeds it from the previous archives, so only the changed debs are fetched. That needs the package lists to exist first, so adding the repository and apt-get update move into their own step ahead of the cache. "chown ... || true" recreated the failure class the assertion exists to catch. apt downloads as _apt, so without the chown the post-job save cannot read the archives. If it fails, the save fails, the cache is never populated, there is never a hit, and the assertion never fires, because it is gated on a hit. It now fails loudly and the directory is checked readable afterwards. Applied to all four cached installs: both ci.yml jobs and both nightly.yml jobs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UqprqkCXuH8SegiZejE1Tw
…mmandprompt#386) The key carried ${{ env.ImageOS }} so a new runner image, which can bring a different LLVM major, would not be served debs built for the old one. It expanded to nothing, every time. ImageOS is a runner environment variable, and the env context only carries variables set by the workflow, job, or step. Observed in the run this branch just produced: Cache restored from key: apt-pgdev-codecs-pg15-Linux-X64--v1 Cache saved with key: apt-pgdev-codecs-pg15-Linux-X64--9ef45bb1777d-v2 The empty component is the double dash. The protection read as present in review and was not there. It now goes into the version hash, computed in a shell step where $ImageOS has a value, and the step prints the image name and the resolved candidate versions so an empty one is visible in the log rather than silently hashing to a constant. Key bumped to v3, since the composition changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UqprqkCXuH8SegiZejE1Tw
jdatcmd
left a comment
There was a problem hiding this comment.
Two of three closed, and I proved the third myself. But the run exposed a fourth.
The hit path now has evidence, and it works
I said the greens were all from the miss path. Your push produced a second run against an
unchanged key, which is the test. From run 31016855942:
build (PG 15, x86_64) Cache restored successfully
build (PG 15, x86_64) Cache restored from key: apt-pgdev-codecs-pg15-Linux-X64--9ef45bb1777d-v2
build (PG 16, aarch64) Cache restored from key: apt-pgdev-codecs-pg16-Linux-ARM64--db7e286b4f4d-v2
build (PG 17, aarch64) Cache restored from key: apt-pgdev-codecs-pg17-Linux-ARM64--5d4622bacd0a-v2
Those are exact key restores, not restore-keys prefix matches, so cache-hit was
true and the assertion was armed. The run succeeded, which means it found no Get: lines.
Cache hit, nothing downloaded, assertion live. That is the property, demonstrated.
test -n "$v" is a better guard than I asked for. A hash of nothing is a constant, and a
constant would silently restore the frozen key the whole change exists to remove. Good
catch.
The chown and the restore-keys comment about prefix matches reporting cache-hit
false are both right.
New, and blocking: env.ImageOS is empty, so the key does not contain it
Look at the keys again:
apt-pgdev-codecs-pg15-Linux-X64--9ef45bb1777d-v2
^^
A double dash where ImageOS should be. ${{ env.ImageOS }} expands to nothing: the
env context holds only variables declared in a workflow, job or step env: block, not
the runner's own environment. ImageOS is a runner environment variable, so it is
reachable as $ImageOS inside a run: step and not through the env context.
The comment says:
ImageOS is in the key because a new runner image can bring a different LLVM major, and
a key that ignored it would serve debs for the wrong one.
That is exactly right, and the key ignores it.
And it reintroduces the false red you just removed. apt-cache policy hashes the five
named packages only, not their transitive dependencies. When GitHub rolls a runner image
with a new LLVM major, those five candidate versions can be unchanged, so the hash is
unchanged, so the key hits exactly. The archives hold debs built against the old LLVM,
apt resolves the new one, downloads it, emits Get:, and the assertion fails the job.
Red matrix on a runner image rollout, which is the same failure mode on a different
trigger.
One extra line in the step that already computes the hash:
v=$(apt-cache policy ... | awk '/Candidate:/ {print $2}' | sort | md5sum | cut -c1-12)
test -n "$v"
echo "k=${ImageOS:-unknown}-$v" >> "$GITHUB_OUTPUT"and drop ${{ env.ImageOS }} from the key template so there is one source for it. Assert
ImageOS is non-empty too, by the same reasoning as test -n "$v": silently folding in
"unknown" is how the protection goes missing a second time.
Worth checking the other three jobs and nightly.yml for the same expansion, since the
key template was copied across all of them.
Everything else here is ready.
…ndprompt#386) The hash now refuses an empty ImageOS the same way it refuses an empty version list. Folding in a placeholder would leave the key well-formed while the runner-image protection was silently gone, which is how it went missing the first time. Observed values on the runners this workflow uses, so the guard is not a tripwire: ubuntu24 on ubuntu-latest, ubuntu24-arm64 on ubuntu-24.04-arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UqprqkCXuH8SegiZejE1Tw
We found ImageOS independently, within a few minutes of each other. It is fixed, andthe hit path now has evidence under the corrected key.I was reading run 31016126558 for the same reason you were, saw Where it went, and why not in the literal keyInto the hash, computed in the step that already shells out, because that is the only The step now reports what it hashed, so an empty one is visible rather than inferred: Both architectures resolve it, so the two arms of the matrix now key differently, which And it fails rather than substituting a placeholderYou asked for The hit path, under the corrected keyRe-run of 31016855942, same commit, so the key is unchanged and the restore is exact: Single dash, hash present, restored key equal to the key the run computes. So The previous run also demonstrated Applied everywhereAll four cached installs: both |
jdatcmd
left a comment
There was a problem hiding this comment.
All four addressed. Verified in a real run, not read.
Run 31018083982:
runner image: ubuntu24 candidates:
cache key hash: 00cf9b764b1d
Cache restored from key: apt-pgdev-codecs-pg15-Linux-X64-00cf9b764b1d-v3
...
cache key hash: d89d52769007
Cache restored from key: apt-pgdev-codecs-pg18-Linux-X64-d89d52769007-v3
ImageOS is populated and inside the hash, the double dash is gone, and the hashes differ
per major, which is what proves the resolved candidate versions are genuinely in there
rather than the key being constant-shaped. Exact-key restores on every job, run green, so
the assertion was armed and found nothing downloaded.
Reading $ImageOS from the shell rather than the expression context is the right fix, and
test -n "${ImageOS:-}" beside test -n "$v" closes it the same way you closed the hash.
Printing both inputs before hashing them is better than either of us proposed: a constant
hash is invisible, and runner image: ubuntu24 in the log is not.
Merging.
One thing I could NOT verify, recorded so nobody assumes I did
I wanted the cold-miss cost, since #386's first bullet is the timeout budget against a
cold fetch. I cannot get it from these runs. The install step reads 5 to 19 seconds in
both the pre-fix and post-fix runs, including the first run under the new key, because
restore-keys matched the older prefix and partially restored. So every run I have is a
hit or a partial hit, and no genuinely cold fetch has been observed since the change.
That is the cache working as designed, and it means the 600 second bound on the miss path
is still untested. I have written this up on #386 rather than leaving it implied here.
Closes #386, implementing the plan from the issue.
What this does
apt writes its archives to
~/apt-archives, whichactions/cachekeys on the PG major,runner OS and arch, and
ImageOS. Four install steps acrossci.ymlandnightly.yml.ImageOSis in the key deliberately: a new runner image can bring a different LLVM major,and a key that ignored it would serve debs for the wrong one.
The half I care about more
On a reported cache hit, the step fails if apt fetched anything over the network.
A cache that populates and never hits looks identical in the log to one that works. The job
is simply still slow and nothing says so. That is the same shape as #396, where the suite
had the exemption and not the invocation, and it is the failure I would otherwise be
shipping here.
What I did not do
Raise the bound. At 30 kB/s the fetch needs about 50 minutes, so that trades a red check
for a runner held most of an hour.
Pin a mirror. The runner picks from its own mirrorlist. Betting on a different one is
the bet we are already losing.
Drop the toolchain.
apt-cache depends postgresql-server-dev-18showsDepends: clang-21andDepends: llvm-21-dev, hard, so no apt flag removes them. jd's viewand mine is that there is no reason to want to: it is a legitimate dependency of
postgresql-server-dev-N.What I could not verify, and how it fails
I cannot run GitHub Actions locally, so the first CI run on this PR is the test. I have
validated what I can: both workflows still parse as YAML, all four install steps route
their archives and tee their output, and no package was dropped from any list.
That last one is not rhetorical. My first patch attempt inserted the tee mid-list:
which would have silently stopped installing
lcovandpython3-pipin the coverage job.Caught by reading the generated file rather than trusting the script. Fixed, and every
package list is asserted intact in the diff.
The failure modes are benign by construction. A miss degrades to exactly today's behaviour.
A stale entry is re-fetched by apt rather than used, because apt validates what is in the
archive directory. So the downside is a slow job, not a wrong one.
Worth watching the first few runs for the hit rate rather than assuming it.